Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mostly link-related (regular) updates #978

Merged
merged 4 commits into from
Feb 7, 2020
Merged

Mostly link-related (regular) updates #978

merged 4 commits into from
Feb 7, 2020

Conversation

jorgeorpinel
Copy link
Contributor

No description provided.

and improve some config file comments
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2020-02-06-brgswwa February 7, 2020 02:35 Inactive
@jorgeorpinel

This comment has been minimized.

@casperdcl

This comment has been minimized.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2020-02-06-brgswwa February 7, 2020 06:56 Inactive
@jorgeorpinel
Copy link
Contributor Author

Cool, link check is working after #979. Thanks @casperdcl !

@jorgeorpinel jorgeorpinel changed the title Regular updates Format and cosmetic link-related (regular) updates Feb 7, 2020
@jorgeorpinel jorgeorpinel marked this pull request as ready for review February 7, 2020 07:01
This was referenced Feb 7, 2020
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2020-02-06-brgswwa February 7, 2020 07:47 Inactive
@jorgeorpinel
Copy link
Contributor Author

As noted in #974 (comment), the link check is currently crashing, preventing the CI checks here from completing ATM. Otherwise this is ready for review.

@jorgeorpinel jorgeorpinel changed the title Format and cosmetic link-related (regular) updates Mostly link-related (regular) updates Feb 7, 2020
@casperdcl
Copy link
Contributor

ok finally got around to reviewing this. Will try to re-fix in a new PR. Not sure why you re-ordered links in exclude-links.txt - probably best to keep sorted.

@casperdcl casperdcl mentioned this pull request Feb 14, 2020
4 tasks
@jorgeorpinel
Copy link
Contributor Author

I think I just sorted them alphabetically by domain name. If you leave a PR review here I'll address in a separate one.

@casperdcl
Copy link
Contributor

hmm you mean you strip http(s):// before sorting? Seems unnecessarily inconvenient. Will be fixed in #997 anyway...

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Feb 15, 2020

And www or other subdomains. But no, whatever is easy, as long as we all know what the sorting is. Piping through GNU sort is fine. Thanks

@jorgeorpinel
Copy link
Contributor Author

p.s. I don't assume the file supports comments (and blank lines)? I.e.

# This is a list of URLs to exclude from scripts/...
# It's ordered with GNU `sort`.

http://127.0.0.1/
...

@casperdcl
Copy link
Contributor

I was nearly going to write comments. They should be fine to add (though strictly speaking aren't ignored yet)

@jorgeorpinel jorgeorpinel deleted the 2020-02-06 branch March 21, 2020 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants